[Skills] Improve installed status in search, add source subtitle, and auto-refresh on install#27443
[Skills] Improve installed status in search, add source subtitle, and auto-refresh on install#27443raulgg wants to merge 3 commits intoraycast:mainfrom
Conversation
|
Thank you for your contribution! 🎉 🔔 @keito4 @DaleSeo @pernielsentikaer @kayotimoteo you might want to have a look. You can use this guide to learn how to check out the Pull Request locally in order to test it. 📋 Quick checkout commandsBRANCH="skills/update-installed-detail-in-search"
FORK_URL="https://github.com/raulgg/raycast-extensions.git"
EXTENSION_NAME="skills"
REPO_NAME="raycast-extensions"
git clone -n --depth=1 --filter=tree:0 -b $BRANCH $FORK_URL
cd $REPO_NAME
git sparse-checkout set --no-cone "extensions/$EXTENSION_NAME"
git checkout
cd "extensions/$EXTENSION_NAME"
npm install && npm run devWe're currently experiencing a high volume of incoming requests. As a result, the initial review may take up to 10-15 business days. |
|
@keito4 @DaleSeo I'm keeping this as a draft for now, but it's ready for you to review. I would love to hear your thoughts and feedback on the change I'm proposing before publishing the PR. The skills CLI has a behavior I dislike when adding skills from different sources that share the same name: it replaces the old skill with the new one without any warning. This can be unexpected, and it also feels surprising in the extension. After the recent change to display the "Installed" badge in search results, I thought it would be helpful to clarify the potential outcomes when trying to install a skill that reuses the name of a currently installed one from a different source. I would appreciate any thoughts on this proposed change, whether you think it's excessive or if you have any suggestions for improvements. |
|
Thanks for sharing your proposal, @raulgg! I agree with the overall direction. I'm glad you're coloring the icons of installed skills green. I was planning to make that change to align with how outdated skills are displayed in the other command. The piece I'd push back on is squeezing the sources next the skill names, which could lead to truncation as shown in the screenshot. Skill names are what users primarily scan for, so I think ellipsing them could create more problems than the duplicate-name confusion we're trying to solve. Collisions are the minority case and can usually be avoided by typing more search terms. Could we explore options that preserve the full skill name? I'm happy to discuss the trade-offs. Just want to make sure we don't fix one DX issue by introducing another. @keito4 Please feel free to chime in if you have any other thoughts. |
|
Thank you for your feedback @DaleSeo! This is the type of feedback I was hoping for. You're right that truncating the skill name is far from ideal, and we must avoid that; so I'll try to find an alternative. I was really hoping to be able to show the source in the list since it's also the default behavior of the CLI when using
Unfortunately, Raycast does not currently provide a method to ensure that the title is not truncated, at least to my knowledge. The only compromise I can think of at this moment is to display the subtitle only when the details are hidden. However, the challenge with this approach is that we still lack a mechanism to persist the mode across sessions (we can discuss wether to add this in a later iteration is worth it or not). For my personal workflow, I would prefer to have the subtitle consistently displayed alongside the skill name, even if that means to have the detail hidden by default, as this would help me identify which skill I wish to navigate to, whether by scrolling through the list or by typing further in the search. However, until we can agree on a more effective solution, I am happy with this solution of displaying it only in full width mode, as I agree that truncating the skill name is a significant drawback.
What do you think? |
|
@raulgg Thanks for thinking this through. Great call on the default change! I'm on board with hiding details. The UI looks cleaner and there's less chance skill names will be truncated. Just for context, we initially made detail-on default because the list itself didn't carry enough information to identify a skill at a glance. I think your subtitle work changes that. Having the source, install count, and conflict indicators in the list view makes scanning much more efficient than inspecting. These are separate pages on skills.sh as well. One UI tweak I'd suggest is using a single indicator instead of two. The hammer icon on the left and the check/warning icon on the right convey the same status. Dropping the right-side indicator and letting the left-side indicator carry status via icon/tint alone would free up the row and keep the install counts aligned. If we switching the default, would it help sidestep the persistence issue? If the default fits most cases, I guess there's less pressure to save state? Correct me if I'm mistaken. |
Perfect! I'll implement this change then.
I actually thought about doing what you suggested, dropping the icon accessory from the list to simplify it and relying solely on the icon's color. However, I considered that having the warning icon is a good way to draw attention from the user. Also, the orange color used to represent the conflicts is the same as that used for updates in the "Manage skills" command. So I was a bit unsure about what would be best.
Yes, I think that persisting the mode would not be necessary in this case. |
You might be overthinking this, @raulgg? I thought we could just change both the icon and its color on the left. I picked the hammer icon, but I don't think it has much meaning. Personally, I find the red "Replace Installed Skill" button is eye-catching enough. I don't feel strongly about my suggestion, so it's your call! 😀 |
I'm probably overthinking it! I'm good with changing the icon too. I'll push these changes soon. |
e4f3584 to
ccf3b43
Compare
| interface SkillDetailViewProps { | ||
| resultSkills: Skill[]; | ||
| initialResultIndex: number; | ||
| onViewedSkillChange: (skillId: string) => void; |
There was a problem hiding this comment.
onViewedSkillChange, resultSkills, and initialResultIndex — resultSkills[initialResultIndex] is only used to get a single Skill, and onViewedSkillChange is declared but not destructured or used. If there's no plan for in-view result navigation, could we simplify the interface to accept skill: Skill directly and drop these three props?
There was a problem hiding this comment.
Good catch! I was trying to implement the navigation on the details view, but ended up causing unexpected behaviors in the list view, so I reverted it. I just didn't do the full cleanup properly 🙈
| toast.style = Toast.Style.Success; | ||
| toast.title = toastOpts.successTitle; | ||
| if (toastOpts.successMessage) toast.message = toastOpts.successMessage; | ||
| await onSuccess?.(result); |
There was a problem hiding this comment.
onSuccess runs inside the same try/catch as operation, so if the post-install refresh (e.g. search API revalidation) throws, the success toast is hidden and replaced with "Failed to install skill" even though the install itself succeeded. Consider running onSuccess outside the try/catch, or catching its errors separately.
There was a problem hiding this comment.
Great catch! The failure to refresh shouldn't block the success toast after install. I have moved the call to onSuccess out of the try-catch.
…ist, and refresh on install
ccf3b43 to
7149c06
Compare
Greptile SummaryThis PR improves the Skills extension's installed-skill detection by replacing name-only matching with source-aware matching via
Confidence Score: 4/5Safe to merge after the false-conflict case for lock-less installs is resolved; the risk of a user accidentally replacing a correctly-installed skill is real but requires both a missing lock entry and the user clicking through a destructive confirmation. One P1 finding (false conflict indicator for skills without lock data) prevents a 5. The fix is localised to useInstalledSkillMatches and does not require reworking the broader architecture. extensions/skills/src/hooks/useInstalledSkillMatches.ts — the conflict/unknown distinction; extensions/skills/src/components/InstalledSkillListItem.tsx — undefined subtitle fallback. Important Files Changed
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
extensions/skills/src/hooks/useInstalledSkillMatches.ts:35-40
**Skills without lock data falsely show as "conflict"**
When a skill has no entry in `.skill-lock.json` (e.g., installed with an older CLI version or after deleting the lock file), `installed.source` is `undefined`, `normalizeGithubSource` returns `undefined`, and the function falls through to `{ type: "conflict" }`. This means a correctly-installed skill would display the orange warning icon and "Installed from source 'Unknown source'" tooltip in search results — the same indicator used for a genuine source conflict — rather than a neutral "installed" or "unknown" state.
A user seeing this warning could be misled into clicking "Replace Installed Skill" and overwriting a skill that was already from the right source. A separate `{ type: "unknown" }` match type with its own neutral indicator would avoid conflating the two cases.
### Issue 2 of 3
extensions/skills/src/components/InstalledSkillListItem.tsx:26
**Subtitle silently disappears for skills without lock data**
`skill.source` is only populated for skills that have a `.skill-lock.json` entry. For skills installed without one, `source` is `undefined` and the subtitle renders as nothing — losing the previous `agentsText` ("claude, copilot") that users could read at a glance. Agent info is still accessible in the detail panel via "Toggle Detail", but the change is a silent regression for anyone with older installs in the Manage Skills view.
### Issue 3 of 3
extensions/skills/src/components/actions/InstallSkillAction.tsx:745-746
**`selectableAgents` filters by name from conflict's agents, but lock agents may differ from available agent names**
`installedAgentNames` comes from `installedMatch.agents`, which is populated from the lock file's agent list for the conflicting skill. If these agent name strings don't exactly match the names returned by `discoverAgents()`, agents that should be selectable could be incorrectly excluded from `selectableAgents`. This seems low-risk given consistent naming, but a mismatch could leave the form with no checkboxes to pick from when replacing.
Reviews (4): Last reviewed commit: "fix: restore skill detail install refres..." | Re-trigger Greptile |
9835abb to
d9b535f
Compare
d9b535f to
947dc76
Compare
| map.set(normalizeSkillIdentityPart(record.name), record); | ||
| } | ||
| return map; | ||
| }, [data]); | ||
|
|
||
| const getInstalledMatch = useCallback( |
There was a problem hiding this comment.
Skills without lock data falsely show as "conflict"
When a skill has no entry in .skill-lock.json (e.g., installed with an older CLI version or after deleting the lock file), installed.source is undefined, normalizeGithubSource returns undefined, and the function falls through to { type: "conflict" }. This means a correctly-installed skill would display the orange warning icon and "Installed from source 'Unknown source'" tooltip in search results — the same indicator used for a genuine source conflict — rather than a neutral "installed" or "unknown" state.
A user seeing this warning could be misled into clicking "Replace Installed Skill" and overwriting a skill that was already from the right source. A separate { type: "unknown" } match type with its own neutral indicator would avoid conflating the two cases.
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/skills/src/hooks/useInstalledSkillMatches.ts
Line: 35-40
Comment:
**Skills without lock data falsely show as "conflict"**
When a skill has no entry in `.skill-lock.json` (e.g., installed with an older CLI version or after deleting the lock file), `installed.source` is `undefined`, `normalizeGithubSource` returns `undefined`, and the function falls through to `{ type: "conflict" }`. This means a correctly-installed skill would display the orange warning icon and "Installed from source 'Unknown source'" tooltip in search results — the same indicator used for a genuine source conflict — rather than a neutral "installed" or "unknown" state.
A user seeing this warning could be misled into clicking "Replace Installed Skill" and overwriting a skill that was already from the right source. A separate `{ type: "unknown" }` match type with its own neutral indicator would avoid conflating the two cases.
How can I resolve this? If you propose a fix, please make it concise.



Description
The problem this addresses is that "Search Skills" command previously treated installed skills as installed by
skillIdonly. That meant multiple search results could show as "Installed" even when they were technically different skills from different sources and only shared the same name. In thereact-doctorexample below, several distinct skills appeared installed while only one was.Current:

Proposed change:


This is risky because the Skills CLI currently overwrites installs for skills with the same name without asking the user to confirm that they understand they are replacing a different source. The UX changes in this PR try to make that behavior visible in the extension and guide users through install decisions before they accidentally replace an existing skill.
Changes include:
owner/repo, as the subtitle in both "Search Skills" and "Manage Skills" commands to distinct skill results from different sources at first glance..skill-lock.json.skillIdis installed from a different source, show a warning indicator, and offer a "Replace Installed Skill" action to better represent the outcome of the action.withSkillActionfor more consistent confirmations, toasts, and failure handling.Checklist
npm run buildand tested this distribution build in Raycastassetsfolder are used by the extension itselfREADMEare located outside the metadata folder if they were not generated with our metadata tool